Skip to content

Conversation

@illegalprime
Copy link
Collaborator

@illegalprime illegalprime commented Jul 5, 2016

This code change is not yet complete, and passes no tests complete, needs docs.

Summary of changes:
a lot, so coming soon

http.0.try_clone().ok().map(Box::new)
} else if let Some(ssl) = self.downcast_ref::<SslStream<HttpStream>>() {
unimplemented!();
} else if let Some(ssl) = self.downcast_ref::<HttpsStream<SslStream<HttpStream>>>() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There must be a better way!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah this doesn't seem ideal really - I can't think of a better way right now though, haha

illegalprime referenced this pull request in hyperium/hyper Jul 18, 2016
BREAKING CHANGE: This breaks a lot of the Client and Server APIs.
  Check the documentation for how Handlers can be used for asynchronous
  events.
@cyderize
Copy link
Member

@illegalprime Thanks for this!

These changes seem good to me, I can't really see anything that I think should be done differently.

Awesome work!

@djc
Copy link

djc commented Sep 12, 2016

@cyderize so what needs to happen to land this? Servo would really like to use this.

@therustmonk
Copy link
Contributor

I quite like this changes.
@cyderize @illegalprime What is the fate of this PR?

@beatgammit
Copy link

Any update on this?

@vi
Copy link
Member

vi commented Jan 18, 2017

@beatgammit, Note: forked crate websocket into websocket-vi. Are you interested in rust-websocket continuation? Do you want me to try fixing and integrating this pull request?

@illegalprime
Copy link
Collaborator Author

@vi @beatgammit yeah I've had quite a year of failures on my part 😢 I am refocusing and would love more help! I guess we should make a list of TODOs like in #27

@illegalprime
Copy link
Collaborator Author

@cyderize I'm pretty close to finishing this up, basically everything from #58 is implemented except for permessage-deflate and mio/tokio. All that's left is docs and tests.

Since this change is rather large I wanted to give you a heads up if you wanted to review all the changes.

@illegalprime illegalprime force-pushed the ws-upgrade branch 2 times, most recently from 8dead61 to 9ef62af Compare April 1, 2017 05:58
@illegalprime illegalprime changed the title Upgrade hyper and other requests to a WebSocket Client Implement _so_ many things, upgrade all deps. Apr 1, 2017
Changes the way clients are created slightly:
 - (slightly, ostensibly) better performance when one knows if they want
   a secure connection or not. This means no match statement redirecting
   calls to their respective SSL or TCP stream.
   These calls have been named:
    - `connect` for insecure connections
    - `connect_secure` for secure connections

 - If one does not know how which connection it wants it can take the
   (not verified by testing) performance hit of putting the underlying
   stream on the heap behind a `Box`. This can be done with:
    - `connect_agnostic`

All calls currently take the custom ToWebSocketUrlComponents but that
should be changed to a `Url` (probably from rust-url).
Both streams and request/stream pairs can be made into a WsUpgrade
struct. also AsTcpStream streams have a method to access the inner tcp
stream. Things left:

 - Finish WsUpgrade struct: add methods and ability to accept or reject
   connections.
 - Implement IntoWs for a hyper serverside request.
The last bit is to extract the NetworkStream from a hyper server-side
request and save that as a Stream, then turn it into an Incoming<(..)>
and validate the request.

Implementing Stream for a NetworkStream is proving complicated, since
because now there is conflicting implementations for many structs
implementing NetworkStream.
This was initially very hard because one cannot clone unsized types and
put them on the stack. I was about to revert to WebSocketStream style
enum-variant-per-sized-type style but I managed to live another day
without all those match blocks.
The validation is done, now the only work is to extract the underlying
stream.
@illegalprime illegalprime merged commit f24a974 into websockets-rs:master Apr 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants